Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit telemetry event upon Repo start #3268

Merged
merged 2 commits into from
Apr 7, 2020

Conversation

binaryseed
Copy link
Contributor

This PR enables telemetry handlers to listen to an event that signals that a new Repo has been started. This feature enables automatic instrumentation for Repos, whose telemetry events are not knowable ahead of time.

@josevalim I tried this out in the New Relic agent and it works quite nicely. The only other thought I had was that perhaps this is a more general pattern that could be recommended by telemetry, which led to the thought that the event name itself could be something like [:telemetry, :broadcast] or something. I'm not totally sure about that, but I do like this pattern.

closes #3266

@@ -169,6 +169,12 @@ defmodule Ecto.Repo.Supervisor do
def init({name, repo, otp_app, adapter, opts}) do
case runtime_config(:supervisor, repo, otp_app, opts) do
{:ok, opts} ->
:telemetry.execute(
[:ecto, :repo, :start],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't use start in here because people may think this is a trace and that eventually we will have a stop/exception. What do you think about init or boot?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I'll change to init to match the action that is actually happening

@josevalim josevalim merged commit fc9061a into elixir-ecto:master Apr 7, 2020
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@binaryseed binaryseed deleted the repo-start-telemetry branch April 7, 2020 17:13
@binaryseed
Copy link
Contributor Author

Awesome! Looking forward to the next release to leverage this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Submit a telemetry event upon Repo start
2 participants